Replace odo analyze command #4330
Conversation
…ers) text Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4330 +/- ##
===========================================
+ Coverage 32.37% 43.94% +11.57%
===========================================
Files 85 96 +11
Lines 6505 7759 +1254
Branches 1349 1660 +311
===========================================
+ Hits 2106 3410 +1304
+ Misses 4399 4349 -50 ☔ View full report in Codecov by Sentry. |
msivasubramaniaan
changed the title
|
I still don't think I get the point of this PR. If we are removing I understand using alizer instead of odo for #4235 , but that's not what this is. |
Hello @datho7561 |
src/odo/odoWrapper.ts
Outdated
| const parse = JSON.parse(cliData.stdout) as AnalyzeResponse[]; | ||
| return parse; | ||
| const parse = JSON.parse(cliData.stdout) as AlizerAnalyzeResponse[]; | ||
| return [[...parse].shift()]; |
There was a problem hiding this comment.
Shift returns the first element. Why not just return the first element (or undefined if there is no element) and change the return type to Promise<AlizerAnalyzeResponse> ?
src/odo/odoWrapper.ts
Outdated
| @@ -194,13 +194,12 @@ export class Odo { | |||
| ); | |||
| } | |||
|
|
|||
| public async analyze(currentFolderPath: string): Promise<AnalyzeResponse[]> { | |||
| public async alizerAnalyze(currentFolderPath: Uri): Promise<AlizerAnalyzeResponse[]> { | |||
There was a problem hiding this comment.
It might be a good idea to move this function to it's own file, since it no longer uses odo, and this file contains wrappers around odo commands
Signed-off-by: msivasubramaniaan <[email protected]>
test/integration/odoWrapper.test.ts
Outdated
| @@ -100,12 +101,12 @@ suite('./odo/odoWrapper.ts', function () { | |||
| }); | |||
|
|
|||
| test('analyze()', async function () { | |||
There was a problem hiding this comment.
@msivasubramaniaan The test('analyze()', async function () { test case should probably also be moved out from test/integration/odoWrapper.test.ts into a separate test, shouldn't it?
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
|
@msivasubramaniaan The related test are still failing: The first one looks like is hanging out or take a very long time to complete, causing the following tests to fail too. But it could be also that some Could you please take a look? |
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
|
@datho7561 and @vrubezhny Moved |
| res.devfileVersion === version && | ||
| res.devfileRegistry === registry.name, | ||
| ), | ||
| return Array.from(compDescriptions).filter(({ name, version }) => { |
There was a problem hiding this comment.
if (foo) {
return true;
}
return false;is the same as
return foo;If you find the first version easier to understand feel free to keep it, but I personally tend to prefer the second version.
Signed-off-by: msivasubramaniaan <[email protected]>
Signed-off-by: msivasubramaniaan <[email protected]>
Ref: #4320
@davthomp
@vrubezhny
Below are the things missed and I don't know how to handle below two items. Let me know if you have any idea.
https://github.com/devfile/alizer doesn't provide any checksum on the release, So the CLI on our extension downloaded each time
alizer devfile command not having "name" key in the response. But odo analyze does
Ex: {
devfile: "java-maven",
devfileRegistry: "Test1",
devfileVersion: "1.3.1",
name: "org-eclipse-lemminx",
}